Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix GetRequestInfo subresource parsing for proxy/redirect verbs #16570

Merged
merged 1 commit into from
Nov 3, 2015

Conversation

liggitt
Copy link
Member

@liggitt liggitt commented Oct 30, 2015

The root apiserver proxy does not pay attention to subresources, only resources, but RequestInfoResolver#GetRequestInfo is incorrectly considering the first segment of the proxied subpath a subresource.

@k8s-github-robot
Copy link

Labelling this PR as size/S

@k8s-github-robot k8s-github-robot added the size/S Denotes a PR that changes 10-29 lines, ignoring generated files. label Oct 30, 2015
@k8s-bot
Copy link

k8s-bot commented Oct 30, 2015

GCE e2e test build/test passed for commit 7b2aec7f3808f5df5c012c3889d0611e5a3c8a80.

@@ -46,6 +46,12 @@ var specialVerbs = map[string]bool{
"watch": true,
}

// specialVerbsNoSubresources contains root verbs which do not allow subresources
var specialVerbsNoSubresources = map[string]bool{
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks like a sets.String

@deads2k
Copy link
Contributor

deads2k commented Oct 30, 2015

Minor comment. lgtm otherwise.

@liggitt
Copy link
Member Author

liggitt commented Oct 30, 2015

made both sets.String

@k8s-bot
Copy link

k8s-bot commented Oct 30, 2015

GCE e2e test build/test passed for commit 56b7c0ddaf80cb7e4b137d0204050c3e441fe06f.

@k8s-bot
Copy link

k8s-bot commented Oct 30, 2015

GCE e2e test build/test passed for commit 600b5e6.

@liggitt
Copy link
Member Author

liggitt commented Oct 31, 2015

@k8s-bot unit test this

@deads2k
Copy link
Contributor

deads2k commented Nov 2, 2015

lgtm

@deads2k deads2k added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Nov 2, 2015
@deads2k deads2k assigned deads2k and unassigned thockin Nov 2, 2015
@k8s-github-robot
Copy link

@k8s-bot test this [submit-queue is verifying that this PR is safe to merge]

@k8s-bot
Copy link

k8s-bot commented Nov 2, 2015

GCE e2e test build/test passed for commit 600b5e6.

@k8s-github-robot
Copy link

@k8s-bot test this [submit-queue is verifying that this PR is safe to merge]

@k8s-bot
Copy link

k8s-bot commented Nov 2, 2015

GCE e2e test build/test passed for commit 600b5e6.

@k8s-github-robot
Copy link

@k8s-bot test this [submit-queue is verifying that this PR is safe to merge]

@k8s-bot
Copy link

k8s-bot commented Nov 2, 2015

GCE e2e test build/test passed for commit 600b5e6.

@alex-mohr
Copy link
Contributor

@k8s-bot unit test this

@k8s-github-robot
Copy link

@k8s-bot test this [submit-queue is verifying that this PR is safe to merge]

@k8s-bot
Copy link

k8s-bot commented Nov 3, 2015

GCE e2e test build/test passed for commit 600b5e6.

@k8s-github-robot
Copy link

Automatic merge from submit-queue

k8s-github-robot pushed a commit that referenced this pull request Nov 3, 2015
@k8s-github-robot k8s-github-robot merged commit f276572 into kubernetes:master Nov 3, 2015
@liggitt liggitt deleted the proxy_request_info branch November 4, 2015 04:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
lgtm "Looks good to me", indicates that a PR is ready to be merged. size/S Denotes a PR that changes 10-29 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

8 participants